-
Notifications
You must be signed in to change notification settings - Fork 20
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(matchExpr): match expression visualizer #914
Conversation
18bfa62
to
e4e0b0c
Compare
Test image available:
|
66f60d7
to
12e0ad3
Compare
Read for review now :D Just a small note for testing with observables that includes Also, you might notice that the graph view sometimes flicks on first renders. This is because I have an effect that tries to fit all nodes into views. So, there is some animation there but too fast to see. |
Test image available:
|
Can that action be removed from the sidebar panel component when it's in this visualization context? |
Yeh sure, of course! I left it in case the user want to add credentials in Create Rule Form. For example. if the target is matched but could not fetch templates, adding credentials there can help refreshing on the go. What do you think? But yeh it really still needs removing when for example, on a credential modal. |
Views appear not responding to target discovery events when adding credentials (i.e. create form rule). Investigating... |
Test image available:
|
Test image available:
|
Test image available:
|
Yea, the Actions menu within the sidecar panel within the modal. Nothing happens when I click it. The "outer" one, in the Topology view, has actions when I click it. |
Also the text at the top of that modal needs updating:
Should be more generic and just talk about opening connections, not specifically JMX ones. |
Ohhh interesting...there is a bug somewhere I think. It should show up. Let me check! |
Test image available:
|
Never mind, the menu was actually showing but it was mounted in the back of the modal haha. Fixed now :D |
Test image available:
|
|
Test image available:
|
Test image available:
|
Looks good testing it out. I'll review the implementation later this afternoon. |
Screencast.from.03-22-2023.03.15.13.PM.webmIf it's not hard to do, keep the positions of the nodes when switching between graph view and list view? |
Should be okay to do I think. Just that this might cause another huge chunk of data to be stored if # of targets is large. Especially, topology view also saves positions too.
https://www.patternfly.org/v4/components/form/design-guidelines No guideline for valid message... I think the green color + check icon should be enough to indicate that. |
Updated to to save graph positions and clean up css, console.logs :)) |
Signed-off-by: Thuan Vo <[email protected]>
Test image available:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great!
Welcome to Cryostat! 👋
Before contributing, make sure you have:
main
branch[chore, ci, docs, feat, fix, test]
git commit --amend --signoff
Fixes: #911
Fixes: #912
Fixes: #900
Description of the change:
Note: No tests yet tho. Focusing on flushing out features for now.
Motivation for the change:
See #376 (comment)
How to manually test:
Run CRYOSTAT_IMAGE=quay.io... sh smoketest.sh...
Screenshots
Credential Modal
Create Rule Form
Match Expression Hint